feat(birmel): replace VoltAgent with explicit AI SDK runtime - #2009
feat(birmel): replace VoltAgent with explicit AI SDK runtime#2009shepherdjerred wants to merge 7 commits into
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
@codex review |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f11538cb9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CASE WHEN "toolId" IS NULL THEN 'message' ELSE 'tool' END, | ||
| CASE WHEN "toolId" IS NULL THEN "naturalDesc" ELSE NULL END, | ||
| "toolId", "toolInput", 'discord', 3, 300000, |
There was a problem hiding this comment.
When production contains a pending ScheduledTask whose toolId is send-message, this migration classifies it as a tool payload and preserves that obsolete ID. The previous migration helper explicitly converted these rows into message payloads, while the new registry contains manage-message but no send-message, so the migrated reminder will retry and eventually fail instead of being delivered. Extract the stored content and migrate this case as payloadKind = 'message' before dropping ScheduledTask.
AGENTS.md reference: packages/birmel/AGENTS.md:L34-L35
Useful? React with 👍 / 👎.
| "manage-scheduled-message", | ||
| metadata("manage-scheduled-message", "messaging", "write"), |
There was a problem hiding this comment.
Registering manage-scheduled-message keeps the legacy ScheduledAnnouncement scheduler exposed alongside the new durable job runtime. Requests routed to messaging can therefore create delayed work that bypasses AgentJob claims, run history, request-context persistence, and retry behavior, despite the package contract designating manage-job as the sole jobs surface. Remove this registration or implement scheduled messages as manage-job message payloads.
AGENTS.md reference: packages/birmel/AGENTS.md:L68-L71
Useful? React with 👍 / 👎.
| const job = await prisma.agentJob.create({ | ||
| data: { |
There was a problem hiding this comment.
When repeated create requests reach SCHEDULER_MAX_TASKS_PER_GUILD (or the recurring-job cap), this path still inserts every job because neither configured scheduler limit is consulted anywhere in the new creation flow. The replaced scheduling surface rejected creation at the guild cap; losing that guard allows an agent or trusted user to grow the durable queue without bound and increases every scheduler scan. Count the applicable active/recurring jobs and reject creation at the configured limits, ideally in the same transaction as the insert.
Useful? React with 👍 / 👎.
PR Summary by Qodofeat(birmel): replace VoltAgent with explicit AI SDK 6 agent runtime
AI Description
Diagram
High-Level Assessment
Files changed (111)
|
Code Review by Qodo
1. Timeout releases job lease
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9ef54d185
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SELECT | ||
| "id", "guildId", "channelId", "threadId", "userId", "channelId", | ||
| "name", "description", "scheduleKind", "scheduleValue", "timezone", | ||
| "nextRunAt", "status", "payloadKind", "message", "toolId", "toolInput", |
There was a problem hiding this comment.
When production contains a pending AgentJob whose toolId is manage-task or manage-scheduled-message, this copy preserves that ID even though the new allTools registry no longer includes either tool, so executeRegisteredTool will fail every run with “Tool not found or not executable.” The fresh migration handles legacy ScheduledTask/send-message rows, but it also needs to translate existing AgentJob payloads or retain compatibility executors for their stable IDs.
AGENTS.md reference: packages/birmel/AGENTS.md:L34-L34
Useful? React with 👍 / 👎.
| claimedAt: null, | ||
| claimedBy: null, | ||
| leaseExpiresAt: null, |
There was a problem hiding this comment.
When cancel targets a currently running job, clearing claimedBy makes both markJobSuccess and markJobFailure fail their status = 'running' AND claimedBy = claimId fence; those functions then return before updating the associated AgentJobRun. The job remains cancelled, but its run history is permanently stuck at running because lease recovery only scans jobs whose status is still running. Either reject cancellation while running or explicitly finalize/fence the active run without discarding its claim bookkeeping.
AGENTS.md reference: packages/birmel/AGENTS.md:L68-L71
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6f56b03fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await failedResponseMessage.edit( | ||
| `I couldn't complete that request. Reference: ${reference}`, |
There was a problem hiding this comment.
When a specialist has already performed its action and the final Discord edit succeeds, a later failure in appendCompletedSessionEvents or completeAgentRun enters this catch and overwrites the successful response with an incident message. The user is then told the request failed even though its write or destructive action may have completed, encouraging a duplicate retry and producing a second final edit; post-delivery persistence failures need handling that preserves the delivered result.
AGENTS.md reference: packages/birmel/AGENTS.md:L23-L25
Useful? React with 👍 / 👎.
| { once: true }, | ||
| ); | ||
| }); | ||
| return await Promise.race([operation(controller.signal), abortPromise]); |
There was a problem hiding this comment.
When a tool ignores the supplied signal—as most existing tools do, including manage-role, whose executor only accepts ctx—this race rejects at the timeout while operation continues running. The agent or durable-job runner can therefore record failure and retry while the original write later completes, duplicating messages, roles, or other side effects; timeout handling must not relinquish the operation until it has actually stopped, or every registered tool must enforce abort-aware cancellation.
Useful? React with 👍 / 👎.
| kind: z.literal("tool"), | ||
| toolId: z.string().min(1), | ||
| input: z.record(z.string(), z.unknown()).default({}), |
There was a problem hiding this comment.
For a tool job, this schema accepts any nonempty ID and arbitrary input, so createAgentJob reports success and persists typos, removed IDs, or arguments that violate the registered tool's Zod schema. Validation occurs only when the delayed execution calls tool.execute, potentially hours later, at which point every attempt fails; creation and editing should resolve the stable registry entry and validate its input before storing the job.
AGENTS.md reference: packages/birmel/AGENTS.md:L34-L35
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6f56b03fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const objectInput = ObjectInputSchema.safeParse(input); | ||
| if (!objectInput.success || !("guildId" in objectInput.data)) { | ||
| return input; |
There was a problem hiding this comment.
When an input schema makes guildId optional, this returns the model input unchanged if the field is omitted rather than deriving the guild from RequestContext. For example, manage-election accepts an omitted guild but then passes undefined to most handlers, so otherwise valid calls fail even though trusted guild context is available; always inject the runtime guild into object inputs rather than requiring the key to be model-supplied.
AGENTS.md reference: packages/birmel/AGENTS.md:L39-L39
Useful? React with 👍 / 👎.
| step.toolResults.map((toolResult) => ({ | ||
| toolId: toolResult.toolName, | ||
| content: `Tool ${toolResult.toolName} completed`, | ||
| })), |
There was a problem hiding this comment.
When a tool returns a validated failure such as { success: false, message: ... }, AI SDK still includes it in step.toolResults, but this hard-codes the event as “completed” and discards the result. Active sessions therefore persist failed writes as successful tool summaries, and later context or summarization can treat an action that never happened as verified; derive a bounded summary/status from the validated tool result instead.
AGENTS.md reference: packages/birmel/AGENTS.md:L63-L65
Useful? React with 👍 / 👎.
| await withTurnQueue(queueId, async () => { | ||
| await processAdmittedTurn(context, run.id); |
There was a problem hiding this comment.
When two thread messages are admitted close together and the first turn archives or cancels the session, the second already holds the old activeSessionId and runs after the first without rechecking session state. This lets a causally later queued turn execute and append events after cancellation, so verify that the session is still active once the queue is acquired before processing it.
AGENTS.md reference: packages/birmel/AGENTS.md:L66-L67
Useful? React with 👍 / 👎.
| prisma.agentJobRun.updateMany({ | ||
| where: { | ||
| jobId: { in: staleIds }, | ||
| status: { in: ["running", "timed_out"] }, | ||
| }, |
There was a problem hiding this comment.
When two recovery callers observe the same expired job, one can recover and re-claim it before the other's transaction starts; this unqualified run update then marks the newly created active run as recovered, even though the following job update rejects its fresh lease. The new execution continues, but success/failure finalization cannot update that run because it is no longer running, corrupting run history; recover only the run associated with the expired claim and only when the matching job transition succeeds.
AGENTS.md reference: packages/birmel/AGENTS.md:L68-L71
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ac41b95d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const config = getConfig(); | ||
| return { | ||
| openai: { | ||
| store: false, |
There was a problem hiding this comment.
When the configured reasoning model invokes a tool, ToolLoopAgent makes a follow-up Responses request, but store: false requires the preceding reasoning item to be returned as encrypted content for that next step. The replaced src/voltagent/openai-provider-options.ts explicitly requested include: ["reasoning.encrypted_content"] for this reason; dropping it here can make every specialist tool call fail after its first step instead of producing a final response. Restore that include option while retaining stateless storage.
AGENTS.md reference: packages/birmel/AGENTS.md:L19-L22
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f42fdb0494
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| AND "legacyAction" = 'schedule' | ||
| AND julianday("legacyScheduledAt") IS NOT NULL | ||
| THEN "legacyScheduledAt" ELSE "nextRunAt" END, | ||
| "status", |
There was a problem hiding this comment.
When upgrading after an ungraceful stop leaves a legacy AgentJob in running, this copy preserves that status even though the legacy schema has no claim fields, so the new row receives claimedBy = NULL. findExpiredAgentJobLeases only selects running jobs with a claim, while normal ticks only select active/retrying jobs; consequently the job and its active run remain stuck forever. Normalize legacy running jobs and runs into a recoverable state during migration.
AGENTS.md reference: packages/birmel/AGENTS.md:L68-L71
Useful? React with 👍 / 👎.
| const fingerprint = readDatabaseFingerprint(database); | ||
| if (!fingerprint.hasMigrationTable && fingerprint.tables.length > 0) { | ||
| verifyBaselineFingerprint(fingerprint); | ||
| shouldResolveBaseline = true; |
There was a problem hiding this comment.
When an existing database was created through the previously committed Prisma migrations, it already has _prisma_migrations, so this condition skips both fingerprint validation and migrate resolve. Because those migration directories were replaced by the new 20260808000000_baseline, migrate deploy then treats that baseline as pending and its first CREATE TABLE fails against the existing schema, preventing startup. Recognize and resolve the verified legacy migration history as well as databases with no migration table.
Useful? React with 👍 / 👎.
| content: result.message, | ||
| delivery, | ||
| }); | ||
| return { data: result.data, delivery }; |
There was a problem hiding this comment.
When Discord delivery returns its normal { success: false, ... } result—for example after the target channel is deleted or becomes inaccessible—this wraps the result under delivery rather than exposing or throwing the failure. The scheduler's toolResultStatus only checks a top-level success, so it marks the isolated-agent run successful, advances its recurrence, and never retries even though no message was delivered. Validate the delivery result before appending the session event and returning.
AGENTS.md reference: packages/birmel/AGENTS.md:L68-L71
Useful? React with 👍 / 👎.
| await appendJobSessionEvent({ | ||
| execution, | ||
| role: "tool", | ||
| eventType: "scheduled-tool-summary", | ||
| content: `Scheduled tool ${job.toolId} completed`, |
There was a problem hiding this comment.
Although the specialist path now records failed tool outcomes, this remaining durable-job path appends Scheduled tool ... completed before the scheduler inspects the returned { success: false }. For a session-bound job whose deterministic tool reports failure, the run is correctly retried or failed afterward, but session history and summaries permanently claim that the action completed, with another false success added on every retry. Derive the event status from the result and only summarize the actual outcome.
AGENTS.md reference: packages/birmel/AGENTS.md:L63-L65
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
Deployment boundary
This remains draft until Buildkite is green. Immediately before production rollout: recheck legacy/runtime row counts, record the current image and environment, create and verify a fresh PVC snapshot, then deploy through the existing image and GitOps flow. Live acceptance must cover conversation, read/write tools, memory lifecycle, a two-turn thread session, a one-shot job, browser/editor health, one reply per input, clean logs/traces, stable context sizes, and no writes to mastra-memory.db.